feat: test retry ui#764
Conversation
✅ Component tests succeed
|
✅ E2E tests succeed
|
sonic16x
force-pushed
the
add-test-retry-ui
branch
3 times, most recently
from
e495cce to
3efd9c9
Compare
commit: |
sonic16x
force-pushed
the
add-test-retry-ui
branch
4 times, most recently
from
2caeb8d to
0d8068b
Compare
sonic16x
force-pushed
the
add-test-retry-ui
branch
8 times, most recently
from
cd1c4c4 to
0323b05
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0323b05899
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (let i = 0; i < repeatCount; i++) { | ||
| await app.run(tests, {retry: repeatCount === 1}); |
There was a problem hiding this comment.
The /run handler now awaits every app.run call before returning 200, which turns this endpoint into a long-lived request for the full test duration (and for all repeats). In environments with request timeouts (browser/network/proxy), this can cause the request to be aborted or retried while tests are still executing, leading to duplicate runs or flaky UI behavior; the previous contract in this handler was explicitly fire-and-forget.
Useful? React with 👍 / 👎.
| const repeatCount = useSelector((state) => state.repeatCount); | ||
|
|
||
| const changeRepeatCount = (newValue: number): void => { | ||
| if (newValue >= MIN_REPEATER_COUNT && newValue < MAX_REPEATER_COUNT) { |
There was a problem hiding this comment.
The guard uses newValue < MAX_REPEATER_COUNT, so MAX_REPEATER_COUNT itself (99) is unreachable from both typing and the increment button. This is an off-by-one bug: users can only set up to 98 even though the constant declares 99 as the limit.
Useful? React with 👍 / 👎.
shadowusr
left a comment
shadowusr
left a comment
There was a problem hiding this comment.
Loooks good to me other than that issue with retry button re-enabling between retries.
| // do not wait for completion so that response does not hang and browser does not restart it by timeout | ||
| app.run(req.body); | ||
| for (let i = 0; i < repeatCount; i++) { | ||
| await app.run(tests, {retry: repeatCount === 1}); |
There was a problem hiding this comment.
What does passing retry change?
Also, the comments in code above and codex's are valid IMO. I've tried running slower tests on a large project and the request takes minutes to complete. I'm afraid it will timeout if I tried to run more tests.
There was a problem hiding this comment.
Fixed both problems
There was a problem hiding this comment.
retry need for disable retry for repeat mode
sonic16x
force-pushed
the
add-test-retry-ui
branch
from
0323b05 to
d5aab1d
Compare
2 participants
No description provided.